-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Make target pointer width in target json an integer #144443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make target pointer width in target json an integer #144443
Conversation
|
This PR modifies cc @jieyouxu These commits modify compiler targets. Some changes occurred in src/tools/compiletest cc @jieyouxu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if it's worth breaking everyone over such a minor improvement.. maybe? maybe not?
This comment has been minimized.
This comment has been minimized.
I'm inclined to say yes, but it would probably be good to get this merged today so it can be in the same nightly as the other breaking change instead of two consecutive nightlies (it would also be fine to do that, but less cool) |
We already did that with #144443... the current state is just inconsistent. But, no strong opinion. |
bb5b5c5
to
283f21b
Compare
@Noratrieb what's the other breaking change? #142352 has been merged for over a month.. |
This comment has been minimized.
This comment has been minimized.
283f21b
to
267f0bd
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Uh, but then why did Miri CI only start failing today? Something changed in 9748d87...b56aaec. |
b64ab4a
to
b2871e5
Compare
The breaking change was probably #144218. |
It might be that the field of wrong type just got ignored, which is exactly why Nora ported this to serde. |
You are right indeed. The code in #142352: if let Some(s) = obj.remove(name).and_then(|b| b.as_u64()) {
base.$key_name = s as $int_ty;
}
|
This comment has been minimized.
This comment has been minimized.
This upgrades the Rust CI from v6.16-rc1 plus a temporary commit for the >= 1.91 target spec [1] to v6.17-rc3 with two temporary commits -- one for the same target spec format change [1] and another for the `file_as_c_str` change [2]. Link: rust-lang#144443 [1] Link: rust-lang#145928 [2] Signed-off-by: Miguel Ojeda <[email protected]>
This upgrades the Rust CI from v6.16-rc1 plus a temporary commit for the >= 1.91 target spec [1] to v6.17-rc3 with two commits pending to be merged upstream -- one for the same target spec format change [1] and another for the `file_as_c_str` change [2]. Link: rust-lang#144443 [1] Link: rust-lang#145928 [2] Signed-off-by: Miguel Ojeda <[email protected]>
This upgrades the Rust CI from v6.16-rc1 plus a temporary commit for the >= 1.91 target spec [1] to v6.17-rc3 with two commits pending to be merged upstream -- one for the same target spec format change [1] and another for the `file_as_c_str` change [2]. Link: rust-lang#144443 [1] Link: rust-lang#145928 [2] Signed-off-by: Miguel Ojeda <[email protected]>
CI: rfl: move job forward to Linux v6.17-rc3 plus 2 commits This upgrades the Rust CI from v6.16-rc1 plus a temporary commit for the >= 1.91 target spec [1] to v6.17-rc3 with two commits pending to be merged upstream -- one for the same target spec format change [1] and another for the `file_as_c_str` change [2]. Link: rust-lang#144443 [1] Link: rust-lang#145928 [2] r? `@lqd` `@Kobzol` try-job: x86_64-rust-for-linux `@rustbot` label A-rust-for-linux `@bors` try
CI: rfl: move job forward to Linux v6.17-rc3 plus 2 commits This upgrades the Rust CI from v6.16-rc1 plus a temporary commit for the >= 1.91 target spec [1] to v6.17-rc3 with two commits pending to be merged upstream -- one for the same target spec format change [1] and another for the `file_as_c_str` change [2]. Link: rust-lang#144443 [1] Link: rust-lang#145928 [2] r? ``@lqd`` ``@Kobzol`` try-job: x86_64-rust-for-linux ``@rustbot`` label A-rust-for-linux ``@bors`` try
Rollup merge of #146154 - ojeda:rfl, r=lqd CI: rfl: move job forward to Linux v6.17-rc3 plus 2 commits This upgrades the Rust CI from v6.16-rc1 plus a temporary commit for the >= 1.91 target spec [1] to v6.17-rc3 with two commits pending to be merged upstream -- one for the same target spec format change [1] and another for the `file_as_c_str` change [2]. Link: #144443 [1] Link: #145928 [2] r? ``@lqd`` ``@Kobzol`` try-job: x86_64-rust-for-linux ``@rustbot`` label A-rust-for-linux ``@bors`` try
…r-width, r=Noratrieb Make target pointer width in target json an integer r? Noratrieb cc `@RalfJung` (https://github.com/rust-lang/rust/pull/142352/files#r2230380120) try-job: x86_64-rust-for-linux
commit 8851e27 upstream. Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 8851e27 upstream. Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 8851e27d2cb947ea8bbbe8e812068f7bf5cbd00b upstream. Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit 17cab7b45f4db7fcc59420407e49ed66494a556c)
Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
* Handle Rust >= 1.91 change to `target-pointer-width` JSON type rust-lang/rust#144443 * Update changelog
commit 8851e27d2cb947ea8bbbe8e812068f7bf5cbd00b upstream. Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit 17cab7b45f4db7fcc59420407e49ed66494a556c)
Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
commit 8851e27d2cb947ea8bbbe8e812068f7bf5cbd00b upstream. Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit 17cab7b45f4db7fcc59420407e49ed66494a556c) Signed-off-by: Harshit Mogalapalli <[email protected]>
* Handle Rust >= 1.91 change to `target-pointer-width` JSON type rust-lang/rust#144443 * Update changelog
Starting with Rust 1.91.0 (expected 2025-10-30), the target spec format has changed the type of the `target-pointer-width` key from string to integer [1]. Thus conditionally use one or the other depending on the version. Cc: Waffle Maybe <[email protected]> Link: rust-lang/rust#144443 [1] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
Just wanted to say, I'm really disappointed that such a trivial breaking charge was merged without any deprecation period and without making it just accept both forms for compatibility (which also would have been trivial), which would save a lot of pain for a lot of people. Not everyone wants to live on the bleeding edge of MSRV, and some people care that their software works on a wide range of rustc versions. |
Hm yeah maybe we should have accepted both forms for a while. OTOH, without a lint hardly anyone would have noticed so there would still have been breakage later when the old form is removed, and a lint would have been more extra work.
We have always been very clear that there is no stability on nightly.
|
Be that as it may, this is just pretty much an aesthetic change with no real practical benefit besides "it's nicer this way", it was "stable" and worked for years (even if not officially stabilized), there's no non-nightly alternative so those who need it are forced to use it, and many people depend on it. Doesn't the cost-benefit ratio here seem very skewed to you here? Sure, it would be extra work to make it temporarily accept both forms and add a lint for a few releases, but that extra work wasn't eliminated, just multiplied and pushed somewhere else, because now everyone who uses this and doesn't want to require a bleeding edge MSRV will now have to do the extra work to autodetect the compiler and support both. Anyway, I apologize if I may seem blunt; I know you're doing a thankless job. I just wish this would have been handled better; I suppose it's too late now to do anything about it? |
r? Noratrieb
cc @RalfJung (https://github.com/rust-lang/rust/pull/142352/files#r2230380120)
try-job: x86_64-rust-for-linux